Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Access] Upgrade lru cache v2 #4700

Merged
merged 17 commits into from
Sep 22, 2023
Merged

[Access] Upgrade lru cache v2 #4700

merged 17 commits into from
Sep 22, 2023

Conversation

nozim
Copy link
Contributor

@nozim nozim commented Sep 9, 2023 •
edited
Loading

Revised version of older PR (#4605) synced from latest master.

@nozim nozim changed the title Upgrate lru cache v2 [Access] Upgrate lru cache v2 Sep 9, 2023
@nozim nozim mentioned this pull request Sep 9, 2023
Closed
@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2023 •
edited
Loading

Codecov Report

Patch coverage: 83.33% and project coverage change: -0.49% ⚠️

Comparison is base (58a36d0) 55.64% compared to head (6324e09) 55.15%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4700      +/-   ##
==========================================
- Coverage   55.64%   55.15%   -0.49%     
==========================================
  Files         932      898      -34     
  Lines       86370    80612    -5758     
==========================================
- Hits        48059    44462    -3597     
+ Misses      34675    32966    -1709     
+ Partials     3636     3184     -452
Flag Coverage Δ
unittests 55.15% <83.33%> (-0.49%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
engine/access/rpc/backend/backend_scripts.go 66.23% <0.00%> (+0.56%) ⬆️
engine/access/rpc/backend/backend_transactions.go 51.06% <ø> (-2.96%) ⬇️
module/trace/trace.go 0.00% <0.00%> (ø)
engine/access/rpc/backend/backend.go 72.30% <50.00%> (+0.21%) ⬆️
engine/consensus/approvals/approvals_lru_cache.go 49.18% <50.00%> (-0.82%) ⬇️
engine/access/rpc/connection/cache.go 95.55% <100.00%> (ø)
engine/access/rpc/connection/node_mock.go 100.00% <100.00%> (ø)
fvm/storage/derived/derived_chain_data.go 90.47% <100.00%> (ø)

... and 44 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nozim nozim changed the title [Access] Upgrate lru cache v2 [Access] Upgrade lru cache v2 Sep 12, 2023
@nozim
Copy link
Contributor Author

nozim commented Sep 12, 2023

@peterargue @jordanschalm @durkmurder kindly help to rerun the ci. 🙏

@nozim
Copy link
Contributor Author

nozim commented Sep 14, 2023

@kc1116 kindly help to rerun the ci. Updated from latest master.

@nozim nozim requested a review from kc1116 September 14, 2023 15:37
@nozim
Copy link
Contributor Author

nozim commented Sep 18, 2023

@peterargue @kc1116 thanks for approval. Will have to wait for updates in master to ensure the tests pass. I will ask you to merge afterwards 🙏

@peterargue
Copy link
Contributor

@nozim looks like TestExecutionEvictingCacheClients is the test that failed, and is also affected by these changes. Have you looked into why it's failing?

@nozim
Copy link
Contributor Author

nozim commented Sep 20, 2023

@peterargue yes checking :)

@nozim nozim requested a review from peterargue September 21, 2023 13:44
@nozim
Copy link
Contributor Author

nozim commented Sep 21, 2023

@peterargue @kc1116 fixed failing test. Kindly re run the ci.

@peterargue peterargue added this pull request to the merge queue Sep 22, 2023
Merged via the queue into onflow:master with commit 0960d3f Sep 22, 2023
35 of 36 checks passed
@nozim nozim deleted the upgrate-lru-cache-v2 branch October 25, 2024 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ramtinms ramtinms Awaiting requested review from ramtinms

@janezpodhostnik janezpodhostnik Awaiting requested review from janezpodhostnik janezpodhostnik is a code owner

@AlexHentschel AlexHentschel Awaiting requested review from AlexHentschel AlexHentschel is a code owner

@durkmurder durkmurder Awaiting requested review from durkmurder durkmurder is a code owner

@jordanschalm jordanschalm Awaiting requested review from jordanschalm jordanschalm is a code owner

@kc1116 kc1116 Awaiting requested review from kc1116

@peterargue peterargue Awaiting requested review from peterargue peterargue is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nozim @codecov-commenter @peterargue @kc1116 @franklywatson @durkmurder